Skip to content

Conversation

odeke-em
Copy link
Contributor

This change plumbs x-goog-spanner-request-id into BatchCreateSessions and asserts that the header is present for that method.

Updates #3537

@odeke-em odeke-em requested review from a team as code owners April 10, 2025 19:59
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner API. labels Apr 10, 2025
@odeke-em odeke-em force-pushed the x-goog-spanner-request-id-plumb-into-BatchCreateSessions branch 2 times, most recently from b9e9734 to f4c6768 Compare April 10, 2025 20:09
@odeke-em odeke-em force-pushed the x-goog-spanner-request-id-plumb-into-BatchCreateSessions branch from f4c6768 to ecb64c7 Compare April 11, 2025 17:55
@odeke-em
Copy link
Contributor Author

@sakthivelmanii @olavloite @rahul2393 @surbhigarg92 kindly help me run the bots here. Thank you.

@odeke-em odeke-em force-pushed the x-goog-spanner-request-id-plumb-into-BatchCreateSessions branch 2 times, most recently from 4bdb739 to e7463fb Compare April 16, 2025 18:25
@rahul2393 rahul2393 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 17, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 17, 2025
@@ -435,14 +466,18 @@ public AsyncTransactionManagerImpl transactionManagerAsync(TransactionOption...

@Override
public ApiFuture<Empty> asyncClose() {
return spanner.getRpc().asyncDeleteSession(getName(), getOptions());
XGoogSpannerRequestId reqId =
this.getRequestIdCreator().nextRequestId(1 /* TODO: channelId */, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.getRequestIdCreator().nextRequestId(1 /* TODO: channelId */, 0);
this.getRequestIdCreator().nextRequestId(1 /* TODO: channelId */, 1);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I manually have been increasing attempts for it. Let's get this merged in as is and in the next round I shall get them all fixed up.

}

@Override
public void close() {
ISpan span = tracer.spanBuilder(SpannerImpl.DELETE_SESSION);
try (IScope s = tracer.withSpan(span)) {
spanner.getRpc().deleteSession(getName(), getOptions());
XGoogSpannerRequestId reqId =
this.getRequestIdCreator().nextRequestId(1 /* TODO: channelId */, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.getRequestIdCreator().nextRequestId(1 /* TODO: channelId */, 0);
this.getRequestIdCreator().nextRequestId(1 /* TODO: channelId */, 1);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I manually have been increasing attempts for it. Let's get this merged in as is and in the next round I shall get them all fixed up.

@odeke-em odeke-em force-pushed the x-goog-spanner-request-id-plumb-into-BatchCreateSessions branch from cd1af14 to 8c48dec Compare May 16, 2025 21:11
@odeke-em odeke-em requested a review from olavloite May 16, 2025 21:46
@odeke-em odeke-em force-pushed the x-goog-spanner-request-id-plumb-into-BatchCreateSessions branch 2 times, most recently from 39fdba9 to 9484419 Compare May 20, 2025 01:46
@olavloite olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 20, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 20, 2025
@odeke-em odeke-em force-pushed the x-goog-spanner-request-id-plumb-into-BatchCreateSessions branch from 9484419 to d08e24c Compare May 22, 2025 03:22
@odeke-em odeke-em force-pushed the x-goog-spanner-request-id-plumb-into-BatchCreateSessions branch from 9608560 to 591ab72 Compare May 26, 2025 07:28
@olavloite olavloite added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels May 26, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 26, 2025
Comment on lines +1090 to +1097
public int hashCode() {
return RequestIdOption.class.hashCode();
}

@Override
public boolean equals(Object o) {
return o instanceof RequestIdOption;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding #3815 (comment)

LastStatementOption does not contain any state, and hence the hashCode and equals methods just returns the hash code of the class / checks if the other object is also an instance of the same class.

This option does contain state, and the hashCode and equals methods should use that state to calculate a hash value and check equality. Let's fix that in a follow-up PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olavloite I've implemented it in PR #3900

@olavloite olavloite merged commit 10c3563 into googleapis:main May 26, 2025
44 of 54 checks passed
@odeke-em odeke-em deleted the x-goog-spanner-request-id-plumb-into-BatchCreateSessions branch May 26, 2025 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-spanner API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants